Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 1-Implement-and-rewrite-test#1259
Scotland | ITP JAN-2026 | Tuan Nguyen | Sprint 3 | 1-Implement-and-rewrite-test#1259Jacknguyen4438 wants to merge 4 commits intoCodeYourFuture:mainfrom
Conversation
…e first course work test
Jacknguyen4438
changed the title
Jacknguyen4438
added
📅 Sprint 3
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
- Function implementation is correct.
Describing tests can be quite challenging. Feel free learn from AI how to keep the test descriptions concise.
Sprint-3/1-implement-and-rewrite-tests/implement/3-get-card-value.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/1-get-angle-type.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/2-is-proper-fraction.test.js
Outdated
Show resolved
Hide resolved
Sprint-3/1-implement-and-rewrite-tests/rewrite-tests-with-jest/3-get-card-value.test.js
Outdated
Show resolved
Hide resolved
cjyuan
added
Reviewed
|
@cjyuan Thank you for the review I have making change base on your PR review. It have been finish and ready to be reviewed again. |
Jacknguyen4438
added
the
Needs Review
| let rank = card.slice(0,-1); | ||
| let suit = card.slice(-1); | ||
| if ( | ||
| typeof card !== "string" || |
There was a problem hiding this comment.
-
If
cardcan be a non-string value, line 42 would have caused an error to be thrown before the type check on line 45. -
If value of
rankandsuitwon't change, it is better to declare them usingconst.
| expect(getAngleType(359)).toEqual("Reflex angle"); | ||
| }); | ||
| // Case 6: Invalid angles | ||
| test(`should return "Invalid angle" when ( 0 < angle && angle > 360)`, () => { |
There was a problem hiding this comment.
The condition, ( 0 < angle && angle > 360), specified in the test description is not quite correct.
Hint: Check if the values you are testing meet the condition.
| test(`should return false when denominator is bigInt`, () => { | ||
| expect(isProperFraction(1, 23443243n)).toEqual(true); | ||
| }); |
There was a problem hiding this comment.
"bigint" is just a data type that can represent any integer, including small integers.
isProperFraction(10, 1n) should return false (if your function can handle "bigint" type value)
| test(`should return false when numerator is infinity`, () => { | ||
| expect(isProperFraction(23432434n, 10)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
23432434n is not the same as infinity. Why test "bigint" type value at all?
| }); | ||
| test(`should return false when denominator < numerator`, () => { | ||
| expect(isProperFraction(1, -2)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
The conditions specified on line 14 and 17 are not how you decide whether a fraction is a proper fraction or not when one the values is negative.
| test(`should return false when numerator is zero`, () => { | ||
| expect(isProperFraction(0, 1)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
Which source are you relying on for the definition of proper fraction?
Shouldn't zero be considered proper fraction?
2 participants
Learners, PR Template
Self checklist
Changelist
I have don't basic understand on how to correctly writing normal test and jest test frame work, I am waiting for this work to be review.
Questions
I have no question.